Skip to content

Conversation

@ChrisRackauckas-Claude
Copy link

Summary

  • Add type parameters to POD struct (POD{S, T <: AbstractFloat}) for improved type stability
  • Add type parameters to SVD and TSVD structs for proper kwargs typing (using NamedTuple)
  • Add return type annotations to key functions for better static analysis
  • Add JET.jl tests to verify type stability of core functions

Changes Made

Type Parameter Improvements

  • POD struct: Changed from Any typed fields to properly parameterized types
    • snapshots::S (parameterized by snapshot type)
    • min_renergy::T, renergy::T (parameterized by element type)
    • rbasis::Union{Missing, Matrix{T}}, spectrum::Union{Missing, Vector{T}}
  • SVD/TSVD structs: Changed kwargs::Any to kwargs::K where K <: NamedTuple

Return Type Annotations

  • matricize(VoV::Vector{Vector{T}})::Matrix{T}
  • determine_truncation(...)::Tuple{Int, T}
  • reduce!(...)::Nothing
  • only_dvs(...)::Bool
  • Base.show(::IO, ::POD)::Nothing

JET.jl Tests

Added new test file test/jet.jl that verifies type stability of:

  • deim_interpolation_indices
  • matricize
  • POD constructors (Matrix and Vector{Vector})
  • reduce! with SVD algorithm

Type Stability Before/After

Before (@code_warntype reduce!(pod, TSVD())):

u::ANY
s::ANY
v::ANY
n_max::ANY

After:

u::Matrix{Float64}
s::Vector{Float64}
v::Matrix{Float64}
n_max::Int64

Test plan

  • Run Pkg.test() - all tests pass
  • Runic formatting check passes
  • JET.jl analysis passes for core functions

CC: @ChrisRackauckas

🤖 Generated with Claude Code

- Add type parameters to POD struct (POD{S, T <: AbstractFloat}) for type stability
- Add type parameters to SVD and TSVD structs for proper kwargs typing
- Add return type annotations to key functions:
  - matricize, determine_truncation, reduce!, only_dvs, Base.show(::IO, ::POD)
- Add JET.jl tests to verify type stability of core functions:
  - deim_interpolation_indices
  - matricize
  - POD constructors
  - reduce! with SVD

The POD struct now properly propagates element types through all operations,
eliminating type instabilities where variables were previously inferred as Any.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ChrisRackauckas-Claude
Copy link
Author

Changes Made

Split JET tests into a separate group excluded from prerelease Julia, following the pattern used by DelayDiffEq and other SciML packages.

Changes:

  1. test/runtests.jl: Updated to use GROUP environment variable pattern

    • Core group: QA, Explicit Imports, POD, utils, DEIM tests
    • JET group: JET static analysis tests
    • All (default): Runs both groups for local development
  2. .github/workflows/Tests.yml: Added group matrix with exclusion

    • Tests now run for both Core and JET groups
    • JET group is excluded from pre (prerelease) Julia version
    • This prevents CI failures when JET has compatibility issues with prerelease Julia

This matches the pattern used in DelayDiffEq.jl where QA tests are excluded from prerelease versions.

- Update test/runtests.jl to use GROUP env pattern (Core and nopre groups)
- Update Tests.yml to run both groups, excluding nopre from prerelease Julia
- JET often has compatibility issues with prerelease Julia versions

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Disable downgrade CI pending dependency updates (see SciML#142)
- Move JET tests to test/nopre/ with separate Project.toml
- Remove JET dependency from main test/Project.toml
- JET tests now only run in nopre group (excluded from pre Julia)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ChrisRackauckas-Claude
Copy link
Author

Additional Changes

Disabled Downgrade CI

Moved JET to separate nopre Project.toml

  • Created test/nopre/Project.toml with JET dependency
  • Moved test/jet.jl to test/nopre/jet_tests.jl
  • Removed JET from main test/Project.toml
  • Updated test/runtests.jl to only run JET tests in the nopre group

This follows the OrdinaryDiffEq pattern where JET has its own Project.toml in a separate test group folder.

- Move qa.jl and explicit_imports.jl to test/nopre/
- Add Aqua and ExplicitImports to test/nopre/Project.toml
- Remove Aqua and ExplicitImports from test/Project.toml
- Update runtests.jl to activate nopre project for nopre group
- nopre group now includes: Aqua, ExplicitImports, JET tests

Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Remove ModelOrderReduction from nopre/Project.toml deps
- Add Pkg.develop(path=...) to use local package instead of registry

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@ChrisRackauckas ChrisRackauckas merged commit f12b0ae into SciML:main Jan 12, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants